Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Kafka and Redis into the infrastructure for both development and production environments, migrating the notification system from local application events to Kafka-based messaging. It also integrates an OpenTelemetry collector for refined trace management, adds Redisson for distributed locking, and extends the reservation slot generation window to 90 days. Key feedback focuses on critical data consistency issues where Kafka messages are sent within database transactions, potentially leading to ghost notifications if a rollback occurs. Additionally, the reviewer identified security risks with root-level container execution, hardcoded broker addresses, and error-handling logic in Kafka consumers that could lead to infinite retry loops.
| @Configuration | ||
| public class KafkaConfig { | ||
|
|
||
| private final String bootstrapServers = "localhost:9092"; |
There was a problem hiding this comment.
Kafka 브로커 주소가 localhost:9092로 하드코딩되어 있습니다. 로컬 개발 환경 외의 Docker Compose나 운영 환경에서는 접근이 불가능할 수 있으므로, @Value를 사용하여 외부 설정 파일(application.yml)로부터 주입받도록 수정해야 합니다.
| private final String bootstrapServers = "localhost:9092"; | |
| @Value("${spring.kafka.bootstrap-servers:localhost:9092}") | |
| private String bootstrapServers; |
| restoreInventory(reservation); | ||
| reservation.changeStatus(ReservationStatus.CANCELED); | ||
| eventPublisher.publishEvent(new ReservationCanceledEvent( | ||
| kafkaTemplate.send("notification.reservation.canceled", new ReservationCanceledEvent( |
There was a problem hiding this comment.
kafkaTemplate.send()가 @Transactional 트랜잭션 범위 내에서 호출되고 있습니다. 만약 이후 로직에서 예외가 발생하여 DB 트랜잭션이 롤백되더라도, Kafka 메시지는 이미 발행되어 알림이 발송되는 데이터 불일치 문제가 발생할 수 있습니다.
이 문제를 해결하기 위해:
- 기존처럼
ApplicationEventPublisher를 사용하여 이벤트를 발행하고, @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)를 사용하는 리스너에서 Kafka 메시지를 전송하도록 구현하는 것을 권장합니다.
| @@ -0,0 +1,60 @@ | |||
| package com.catchtable.config; | |||
There was a problem hiding this comment.
KafkaConfig 클래스가 com.catchtable.config 패키지에 위치하고 있습니다. 프로젝트 내 다른 설정 클래스들(RedissonConfig, MetricsConfig)은 com.catchtable.global.config 패키지에 모여 있으므로, 일관성을 위해 패키지 위치를 통일하는 것이 좋습니다.
| kafka: | ||
| image: apache/kafka:3.7.0 | ||
| container_name: catchtable-kafka-prod | ||
| user: "0" # 볼륨 마운트 폴더에 로그를 쓰고 지울 수 있도록 최고 관리자 권한 부여 |
There was a problem hiding this comment.
보안상 컨테이너를 root 권한(user: "0")으로 실행하는 것은 권장되지 않습니다. 호스트 시스템의 볼륨 권한 문제가 있다면, 적절한 UID/GID를 지정하거나 Dockerfile에서 권한 설정을 처리하는 방식이 더 안전합니다.
| .orElseThrow(() -> { | ||
| log.error("[Kafka Consumer] 사용자 정보를 찾을 수 없습니다. userId={}", userId); | ||
| return new CustomException(ErrorCode.USER_NOT_FOUND); | ||
| }); |
There was a problem hiding this comment.
사용자를 찾지 못할 경우 CustomException을 던지고 있습니다. Kafka 리스너에서 예외가 발생하면 기본적으로 메시지 처리를 재시도하게 되는데, 존재하지 않는 사용자에 대한 이벤트는 재시도해도 성공할 수 없으므로 불필요한 리소스 소모나 무한 재시도가 발생할 수 있습니다. 비즈니스적으로 유효하지 않은 데이터인 경우 로깅 후 메시지를 정상 소모 처리하거나 Dead Letter Topic으로 보내는 처리가 필요합니다.
| @Value("${spring.data.redis.host:localhost}") | ||
| private String host; | ||
|
|
||
| @Value("${spring.data.redis.port:6379}") |
There was a problem hiding this comment.
Redis 연결 정보에 localhost와 6379가 기본값으로 설정되어 있습니다. 운영 환경에서 환경 변수 설정이 누락되었을 때 의도치 않게 로컬 환경으로 접속을 시도하게 되어 장애 원인 파악이 어려워질 수 있습니다. 필수 설정값은 기본값을 제거하여 설정 누락 시 애플리케이션 구동 단계에서 즉시 인지할 수 있도록 하는 것이 안전합니다.
4 participants
📢 기능 설명
필요시 실행결과 스크린샷 첨부
연결된 issue
연결된 issue를 자동을 닫기 위해 아래 {이슈넘버}를 입력해주세요.
close #{이슈넘버}
✅ 체크리스트